Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leaks #3199

Merged
merged 7 commits into from
May 26, 2020
Merged

Fix memory leaks #3199

merged 7 commits into from
May 26, 2020

Conversation

jshier
Copy link
Contributor

@jshier jshier commented May 21, 2020

Goals ⚽

This PR fixes several memory leaks, mostly around DataStreamRequest (😬) and within our test suite.

Implementation Details 🚧

Fixes were accomplished using unowned captures to ensure there are no memory cycles. Our tests had cycles due to strong captures in ClosureEventMonitor closures to requests or Sessions.

Testing Details 🔍

No new specific tests, but I did add a standalone test case, LeaksTests, that has a single method which registers an atexit handler which runs the leaks tool. This allows for semi-automated leaks testing using SPM. Just call swift test -c debug -Xswiftc -DLEAKS in the Alamofire directory and it will run the test suite (or as much of it as it can using SPM) and then run the leaks tool afterward, printing any leaks. The memory debugger in Xcode can also be used to find leaks.

Copy link
Member

@cnoon cnoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @jshier 👌🏻

@jshier jshier merged commit 2406691 into master May 26, 2020
@jshier jshier deleted the bug/memory-leaks branch May 26, 2020 00:34
@jshier jshier added this to the 5.2.1 milestone May 26, 2020
@DenTelezhkin
Copy link

@jshier Apologies for out-of-nowhere question, but how are you able to run this leaks testing?

In my environment(Xcode 11.5, Mac OS Catalina 10.15.4) running swift test -c debug -Xswiftc -DLEAKS in Alamofire directory eventually crashes leaks command line tool with following crashlog: https://pastebin.com/Qza7y4Qv

@jshier
Copy link
Contributor Author

jshier commented May 26, 2020

@DenTelezhkin The libswiftRemoteMirror42.dylib in your crash log seems suspicious, you may want to double check the version of the command line tools you have installed and the version of Xcode you have selected for command line use.

@DenTelezhkin
Copy link

DenTelezhkin commented May 27, 2020

This truly was suspicious, but I have only one Xcode installed, and correct version of command line tools(11.5, the only present) is selected.

The mentioned libswiftRemoteMirror42.dylib file, along with libswiftRemoteMirror.dylib and libswiftRemoteMirrorLegacy.dylib are not even a part of Xcode, but rather part of Mac OS Catalina install - they reside in /usr/lib/swift even if you don't have Xcode installed.

Are you running those commands on Mac OS Catalina yourself?

@jshier
Copy link
Contributor Author

jshier commented May 27, 2020

Yes, it works fine. Can you run leaks at all?

@DenTelezhkin
Copy link

Yep, it sure does, running on my app written in SwiftUI:

> leaks 98795
Process:         Redacted [98795]
Path:            /Users/USER/Library/Developer/Xcode/DerivedData/Redacted-eohepbsmfjzzprdjbskucteuqsys/Build/Products/Debug-maccatalyst/Redacted.app/Contents/MacOS/Redacted
Load Address:    0x100000000
Identifier:      com.Redacted.Redacted
Version:         1.0 (2020.05.22.1004)
Code Type:       X86-64
Parent Process:  debugserver [98796]

Date/Time:       2020-05-28 11:15:36.696 +0300
Launch Time:     2020-05-28 11:14:34.023 +0300
OS Version:      Mac OS X 10.15.4 (19E287)
Report Version:  7
Analysis Tool:   /Applications/Xcode.app/Contents/Developer/usr/bin/leaks
Analysis Tool Version:  Xcode 11.5 (11E608c)

Physical footprint:         39.4M
Physical footprint (peak):  39.8M
----

leaks Report Version: 4.0
Process 98795: 85126 nodes malloced for 10968 KB
Process 98795: 0 leaks for 0 total leaked bytes.

rain2540 added a commit to rain2540/Alamofire that referenced this pull request Aug 4, 2020
* origin/master: (193 commits)
  Use headers property. (Alamofire#3264)
  Prepare 5.2.2 Release (Alamofire#3262)
  Explicitly link CFNetwork when using SPM (Alamofire#3259)
  A Better debugDescription (Alamofire#3256)
  Add macOS(Catalyst) user agent. (Alamofire#3236)
  Temporarily disable redirect tests due to HTTPBin issue (Alamofire#3244)
  Replace if with switch in URLEncoding (Alamofire#3214)
  DataStreamRequest and test reliability fixes (Alamofire#3216)
  Open shouldRetry in RetryPolicy (Alamofire#3181)
  Prepare 5.2.1 (Alamofire#3208)
  Update .background QoS to .utility. (Alamofire#3207)
  Fix memory leaks (Alamofire#3199)
  Fix link in README.
  Prepare 5.2.0 Release (Alamofire#3189)
  More Flexible Response Handlers (Alamofire#3188)
  Minor documentation updates (Alamofire#3187)
  Fix Decimal encoding in URLEncodedFormEncoder. (Alamofire#3185)
  Fix threading issue: activeRequests could be accessed off of the rootQueue. (Alamofire#3179)
  Combine Support! (Alamofire#3160)
  Added AuthenticationInterceptor and Authenticator protocol (Alamofire#3164)
  ...

# Conflicts:
#	.travis.yml
#	Alamofire.podspec
#	Alamofire.xcodeproj/project.pbxproj
#	Alamofire.xcodeproj/xcshareddata/xcschemes/Cleanup Whitespace.xcscheme
#	CHANGELOG.md
#	Documentation/AdvancedUsage.md
#	Example/Source/AppDelegate.swift
#	Example/Source/MasterViewController.swift
#	Example/iOS Example.xcodeproj/project.pbxproj
#	Example/iOS Example.xcodeproj/xcshareddata/xcschemes/iOS Example.xcscheme
#	Gemfile.lock
#	LICENSE
#	Package.swift
#	README.md
#	Source/Info.plist
#	Source/NetworkReachabilityManager.swift
#	Source/Request.swift
#	Source/Response.swift
#	Source/Result.swift
#	Source/ServerTrustPolicy.swift
#	Source/SessionManager.swift
#	Source/TaskDelegate.swift
#	Source/Timeline.swift
#	Source/Validation.swift
#	Tests/BaseTestCase.swift
#	Tests/NetworkReachabilityManagerTests.swift
#	Tests/ResponseSerializationTests.swift
#	Tests/ResultTests.swift
#	Tests/ServerTrustPolicyTests.swift
#	Tests/SessionManagerTests.swift
#	docs/Classes.html
#	docs/Classes/DataRequest.html
#	docs/Classes/DisabledEvaluator.html
#	docs/Classes/DownloadRequest.html
#	docs/Classes/DownloadRequest/Options.html
#	docs/Classes/MultipartFormData.html
#	docs/Classes/MultipartUpload.html
#	docs/Classes/NetworkReachabilityManager.html
#	docs/Classes/NetworkReachabilityManager/ConnectionType.html
#	docs/Classes/NetworkReachabilityManager/NetworkReachabilityStatus.html
#	docs/Classes/Request.html
#	docs/Classes/Request/ValidationResult.html
#	docs/Classes/SessionDelegate.html
#	docs/Classes/SessionManager.html
#	docs/Classes/SessionManager/MultipartFormDataEncodingResult.html
#	docs/Classes/UploadRequest.html
#	docs/Enums.html
#	docs/Enums/AFError.html
#	docs/Enums/AFError/MultipartEncodingFailureReason.html
#	docs/Enums/AFError/ParameterEncodingFailureReason.html
#	docs/Enums/AFError/ResponseSerializationFailureReason.html
#	docs/Enums/AFError/ResponseValidationFailureReason.html
#	docs/Enums/HTTPMethod.html
#	docs/Enums/Result.html
#	docs/Enums/ServerTrustPolicy.html
#	docs/Extensions.html
#	docs/Extensions/Bundle.html
#	docs/Extensions/DispatchQueue.html
#	docs/Extensions/Notification.html
#	docs/Extensions/Notification/Key.html
#	docs/Extensions/Notification/Name.html
#	docs/Extensions/Notification/Name/Task.html
#	docs/Extensions/SCNetworkReachabilityFlags.html
#	docs/Extensions/SecTrust.html
#	docs/Extensions/String.html
#	docs/Extensions/URL.html
#	docs/Extensions/URLComponents.html
#	docs/Extensions/URLRequest.html
#	docs/Functions.html
#	docs/Protocols.html
#	docs/Protocols/DataResponseSerializerProtocol.html
#	docs/Protocols/DownloadResponseSerializerProtocol.html
#	docs/Protocols/ParameterEncoding.html
#	docs/Protocols/RequestAdapter.html
#	docs/Protocols/RequestRetrier.html
#	docs/Protocols/SessionStateProvider.html
#	docs/Protocols/URLConvertible.html
#	docs/Protocols/URLRequestConvertible.html
#	docs/Structs.html
#	docs/Structs/DataResponse.html
#	docs/Structs/DownloadResponse.html
#	docs/Structs/HTTPMethod.html
#	docs/Structs/JSONEncoding.html
#	docs/Structs/URLEncoding.html
#	docs/Structs/URLEncoding/ArrayEncoding.html
#	docs/Structs/URLEncoding/BoolEncoding.html
#	docs/Structs/URLEncoding/Destination.html
#	docs/Typealiases.html
#	docs/badge.svg
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/DataRequest.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/DisabledEvaluator.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/DownloadRequest.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/DownloadRequest/Options.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/MultipartFormData.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/MultipartUpload.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/NetworkReachabilityManager.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/NetworkReachabilityManager/ConnectionType.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/NetworkReachabilityManager/NetworkReachabilityStatus.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/Request.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/Request/ValidationResult.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/SessionDelegate.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/SessionManager.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/SessionManager/MultipartFormDataEncodingResult.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Classes/UploadRequest.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Enums.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Enums/AFError.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Enums/AFError/MultipartEncodingFailureReason.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Enums/AFError/ParameterEncodingFailureReason.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Enums/AFError/ResponseSerializationFailureReason.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Enums/AFError/ResponseValidationFailureReason.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Enums/HTTPMethod.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Enums/Result.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Enums/ServerTrustPolicy.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/Bundle.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/DispatchQueue.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/Notification.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/Notification/Key.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/Notification/Name.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/Notification/Name/Task.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/SCNetworkReachabilityFlags.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/SecTrust.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/String.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/URL.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/URLComponents.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Extensions/URLRequest.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Functions.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Protocols.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Protocols/DataResponseSerializerProtocol.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Protocols/DownloadResponseSerializerProtocol.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Protocols/ParameterEncoding.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Protocols/RequestAdapter.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Protocols/RequestRetrier.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Protocols/SessionStateProvider.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Protocols/URLConvertible.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Protocols/URLRequestConvertible.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Structs.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Structs/DataResponse.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Structs/DownloadResponse.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Structs/HTTPMethod.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Structs/JSONEncoding.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Structs/URLEncoding.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Structs/URLEncoding/ArrayEncoding.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Structs/URLEncoding/BoolEncoding.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Structs/URLEncoding/Destination.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/Typealiases.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/index.html
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/search.json
#	docs/docsets/Alamofire.docset/Contents/Resources/Documents/undocumented.json
#	docs/docsets/Alamofire.docset/Contents/Resources/docSet.dsidx
#	docs/docsets/Alamofire.tgz
#	docs/docsets/Alamofire.xml
#	docs/index.html
#	docs/search.json
#	docs/undocumented.json
#	watchOS Example/watchOS Example WatchKit App/Info.plist
#	watchOS Example/watchOS Example WatchKit Extension/HostingController.swift
heckj added a commit to heckj/OpenTelemetryModels that referenced this pull request Oct 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants